Conversation
…) has been received unless bit 7 is set. fix - codestyle and constants
…er (Arduino - not tested)
…ingle native buffer for NET and Java. No memory copying required
| catch (Exception ex) | ||
| { | ||
| Logger.Error($"[USBDRIVER]: crash {ex}"); | ||
| return; |
There was a problem hiding this comment.
Is it better to throw it normally
There was a problem hiding this comment.
if we call "throw;" instead "return;" exception will be bubbled in awaiter task , i.e. StopProcessingTasks->DeinitBuffersAsync and break them.
This can be done, but I think there is no point in causing a failure with Exception when closing the port.
... or I didn't understand, please explain
There was a problem hiding this comment.
Throw the exception outward and catch it at the final call site, so that the stack trace is not hidden.
There was a problem hiding this comment.
When physically disconnected, both tasks (_receiveTask, _sendTask) may fail with exceptions.
When attempting to close a port, an exception will be thrown in StopProcessingTasks, which will prevent the port from being closed. Port must be "closable" and release the resource in any case, even if some errors occur.
There was a problem hiding this comment.
I did it: Default closing or Disposing the port will still ignore errors and free the port.
Closing a port with parameters (List? errors) allows you to get the entire stack trace and close the port. 30f2281
|
Thank you for your contribution. This is a significant submission. As work has taken up a lot of my time, I will address these submissions as soon as possible. Thank you again. |
I understand you. I also have a lot of work. There's no need to rush. |
| @@ -0,0 +1,12 @@ | |||
| using System; | |||
There was a problem hiding this comment.
It is not recommended to integrate logging internally. Instead, exceptions can be thrown outward to preserve the stack trace, and handled by the user at the final call site.
There was a problem hiding this comment.
In most cases, I needed logging when debugging, but Debug.WriteLine is very slow(Console.WriteLine much faster), which led to delays, GC and the buffer quickly overflowed.
Which option do you prefer?
- We can remove logging and replace it with Debug.WriteLine where possible now
- replace to DI Microsoft.Extensions.Logging.
There was a problem hiding this comment.
ok :-)
I had time to think and I did it - logging removed 5f1cb60
| catch (Exception ex) | ||
| { | ||
| Debug.WriteLine(ex); | ||
| Logger.Error($"[USBDRIVER]: crash {ex}"); |
There was a problem hiding this comment.
A lot of stack trace information is being hidden in the code, which is quite problematic.
| protected Task? _receiveTask; | ||
| protected Task? _sendTask; | ||
|
|
||
| Channel<UsbRequest>? _writeChannel; |
There was a problem hiding this comment.
For most hardware, simultaneous write and read operations are not supported. Communication is typically request–response based, where the next request can only be sent after a reply is received. As a result, using a queue does not bring much benefit and instead makes the code more complex.
There was a problem hiding this comment.
- "Most equipment", but not all. All UART converters are asynchronous.
- I tested, without dedicated tasks and queues, the processing speed drops significantly.
- In Windows/Linux there is buffering, the driver does this there, in our case no one did this.
for example, when working with ch340 or FTDI without such an implementation, the user will have to launch and maintain the reading thread themselves so as not to lose data.
Not everyone has enough experience to understand how to implement this correctly, perhaps for this reason there is still no public working version and many are faced with data loss. UPDATE: readqueue already in master branch.
My implementation ReadAsync WriteAsync - hi-perfomance, easy to use, do not require knowledge of threads, features of Android etc.
There was a problem hiding this comment.
Neither Socket nor SerialPort has built-in queue processing; they only provide read and write functionality. Data queuing should be implemented as an extension of the communication library rather than being built in.
There was a problem hiding this comment.
it's not the case, linux and windows simplify access to the UART by hiding the hardware implementation, queues and buffering, and it works conveniently.
Do you want to reject PR or do you want to move the buffering to a separate extension wrapper class?
Extension will require a lot of my time to develop, testing and coordinate the requirements with you.
| { | ||
| if (ReadHeaderLength < ((NetDirectByteBuffer?)dataRq.ClientData!).Position) | ||
| { | ||
| await dataRqWriter.WriteAsync(dataRq, ct); |
There was a problem hiding this comment.
Whether processing the received data directly is better than first enqueueing the data and then dequeuing it again for processing.
There was a problem hiding this comment.
I tested: the task with UsbDeviceConnection.RequestWaitAsync should work as quickly as possible and mainly deal with waiting. If we start processing data directly in the same task, the data will begin to be delayed, the OS will quickly fill the buffers and the data will “begin to be lost”.
Second important note: a single android lock is used to enqueue and dequeue the linux buffer (in android line 115 ). This means that if we try to receive and send data from multiple threads, there will be a lot of blocking operations on that mutex.
…s, now all exceptions bubbled to CloseAsync | ReadAsync | WriteAsync
|
I just ran into a problem with CH340. @alex3696 Have you been able to solve this problem? |
Yes, I am solving exactly this problem. |
|
@alex3696 Tested the latest version in the MAUI project. At a polling interval of 100ms, everything works flawlessly, and no packet loss or connection issues have been observed. Great improvement, especially under continuous data flow. 👏👏👏 |
In my PR and repository you can see an example of using exchange in a background task without polling interval. In general, I was surprised that there hasn’t been a working solution for so many years. I hope we can come to an agreement and I will have time to formalize the owner’s wishes. |
| catch (Exception ex) | ||
| { | ||
| errors?.Add(ex); | ||
| } |
There was a problem hiding this comment.
Adding a try-catch block here is completely unnecessary. Any exception thrown after await will automatically propagate upward, and the stack trace will be preserved.
There was a problem hiding this comment.
If we don't catch this exceptions, it will stop closing and propagate to Dispose - all will be broken.
Closing the port must occur without fail; it cannot be broken by exceptions.
Yes :-) this is a big pull request with breaking changes.
Reasons:
Task ReadAsync(byte[] dstBuf, int offset, int count, CancellationToken ct)
Task WriteAsync(byte[] srcBuf, int offset, int count, CancellationToken ct)
what else is new:
3. ILogger - added simple custom loggerSo... i did it, and test all. Seems to work well in tests with my listed devices. Testing at high speeds with closed RX-TX lines on the FT232R and CP2102N. The speed depends on the phone and the converter, tested on samsung s24+, a50 , blackview 5900

IO rate is close to BaudRate, and depends on the phone's performance and converter model.